-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update to aframe master build (post 1.6.0, three r167) to fix undo material src #842
Conversation
Ah okay it's when it's creating the scene from streetmix json the calculated position for the ground differ. |
The water should be at y -3 3dstreet/src/aframe-streetmix-parsers.js Line 1796 in a425a6d
adding there a console.log('groundParentEl', groundParentEl.object3D.position); I get 0 0 0 .Same with groundParentEl.setAttribute('position', { x: 0, y: -3, z: 0 }); or groundParentEl.setAttribute('position', '0 -3 0'); If I do groundParentEl.object3D.position.setY(-3); it works. wow. That's new. |
Oh the change of position was synchronous in aframe 1.5.0. In aframe 1.6.0 and master (almost 1.7.0 really), the change is done at the end of tick. groundParentEl.setAttribute('position', { y: -3 });
setTimeout(() => {
console.log('groundParentEl', groundParentEl.object3D.position);
}) shows because 3dstreet/src/aframe-streetmix-parsers.js Line 1815 in a425a6d
override completely the value that will be used at the end |
In aframe 1.5.0
gives
In aframe 1.6.0
The latest setAttribute call wins. I guess we should review the code were we use an object with only x or y and set the position with groundParentEl.object3D.position directly. |
no the setTimeout was already needed in aframe 1.5.0. |
@kfarr you need to explain to me how we get x -30.55 for the left side at the end. Both side are positive: 3dstreet/src/aframe-streetmix-parsers.js Lines 1811 to 1816 in a425a6d
I really don't see were the x coordinate is flipped afterwards. |
So the behavior of using partial object was never intended to work and it's not documented https://aframe.io/docs/1.6.0/components/position.html#main and here it works on aframe 1.5.0 only because it's not attached to the DOM yet. If it's attached to the DOM, setting the position with a partial object will have the other coordinates set to 0. let e = document.createElement('a-entity')
e.setAttribute('position', { y: -3 });
e.setAttribute('position', { x: 30.55 });
AFRAME.scenes[0].appendChild(e)
e.object3D.position
gives {x: 30.55, y: -3, z: 0} only on aframe 1.5.0 but that was an undocumented behavior
gives {x: 30.55, y: 0, z: 0} on aframe 1.6.0
let e = document.createElement('a-entity')
AFRAME.scenes[0].appendChild(e)
e.setAttribute('position', { y: -3 });
e.setAttribute('position', { x: 30.55 });
e.object3D.position
gives {x: 30.55, y: 0, z: 0} on both aframe 1.5.0 and 1.6.0 So we need to modify the code to use el.object3d.position.setY or setX instead or prepare an object and set it with setAttribute('position', obj) in one go. There may be also other part in the code were we set rotation with partial object as well we need to review. |
Thanks for finding this. Yes in general the streetmix-parser logic is sloppy and written a long time ago using a variety of techniques. It does need refactoring, thanks for helping with this. I will test now but I assume somehow through magic the object3d is available to set prior to adding the entity to the dom. |
No magic here, the object3d is created right away when you create a-entity |
update to aframe master build (post 1.6.0, three r167) to fix undo material src (fix #840)